Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for cutting in Safari when selecting all with widget at the beginning #3609

Closed
wants to merge 3 commits into from
Closed

Fix for cutting in Safari when selecting all with widget at the beginning #3609

wants to merge 3 commits into from

Conversation

Comandeer
Copy link
Member

@Comandeer Comandeer commented Oct 30, 2019 •
edited
Loading

What is the purpose of this pull request?

Bug fix

Does your PR contain necessary tests?

All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.

This PR contains

  • Unit tests
  • Manual tests

What is the proposed changelog entry for this pull request?

* [#3537](https://github.com/ckeditor/ckeditor4/issues/3537): [Safari] Fixed: can't cut when the whole content is selected and widget is at the beginning of it.

What changes did you make?

I've manually added widgetselection fillers just for Safari.

Closes #3537.

@f1ames f1ames self-requested a review December 3, 2019 11:50
@f1ames f1ames self-assigned this Dec 3, 2019
@f1ames
Copy link
Contributor

f1ames commented Dec 3, 2019

Rebased onto latest master.

f1ames
f1ames suggested changes Dec 3, 2019
View reviewed changes
Copy link
Contributor

@f1ames f1ames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one issue with the current solution, after cutting there is additional empty paragraph (<p></p>) left in the content. When you paste back cut content, you are not able to select all again because of the <p></p> at the content end. It also pollutes getData(), etc.
It looks like a little broader issue, related to getClipboardHtml(), but still it worked before on Safari (select all + cut) so should be investigated.

safari cut

@Comandeer Comandeer self-assigned this Dec 3, 2019
@Comandeer
Copy link
Member Author

It seems like a deeper issue. I've extracted it to #3705.

@f1ames f1ames mentioned this pull request Dec 4, 2019
Open
@f1ames
Copy link
Contributor

f1ames commented Jan 9, 2020

#3705 was merged to major branch so we can restore work on this PR 🎉

@Comandeer Comandeer changed the base branch from master to major January 12, 2020 18:41
@Comandeer
Copy link
Member Author

I've rebased the branch onto latest major, but the issue with additional, empty p is still present 🤔. Seems like there is some other source of the issue.

@Comandeer
Copy link
Member Author

The issue is connected with the filler itself. Safari can't delete it properly after cutting, leaving its content (space) inside the editor. Editor converts it into the paragraph, which results in two paragraphs (one from the filler and default one, created automatically after cutting).

It seems like our filler needs its own filler to be deleted 😐

@f1ames
Copy link
Contributor

f1ames commented Jan 13, 2020

Widgetselection plugin is quite complicate and so we could try to look for some improvements there. The first thing to check will be trying to simplify it according to issue reported some time ago - #1670.

If we could use the solution proposed there, the issue should go away and we will be able to proceed with this PR.

@Comandeer Comandeer removed their assignment Feb 23, 2020
@jacekbogdanski jacekbogdanski added the pr:frozen ❄ The pull request on which work was suspended due to various reasons. label Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@f1ames f1ames f1ames requested changes

Assignees
No one assigned
Labels
pr:frozen ❄ The pull request on which work was suspended due to various reasons.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot cut after select all when widget on content beignning
3 participants
@Comandeer @f1ames @jacekbogdanski